-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update to embedded-hal
1.0.
#16
Conversation
3cd5be3
to
fa3ca00
Compare
embedded-hal
1.0.embedded-hal
1.0.
95b813a
to
1d22965
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your work.
However, this is way too big for me to review as-is.
Could you split this into smaller PRs?
56fecb6
to
5f797bc
Compare
@eldruin, I moved the additional changes to a separate PR. This one should be manageable now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still changes many things on top of updating to e-h 1.0.
- Please do not remove
DynamicOneShot
. It is a useful trait asked for by the community. - Why did you remove the whole device-specific channel selection mechanism? That ensures that only valid channels for the current device can be read.
From reading #10 (comment), it looks like this trait didn't actually solve the issue, which is why I removed it.
Ah, I missed that this ensures only valid channels. I simply removed it since there is no corresponding |
b58cc94
to
6c12cc8
Compare
@eldruin, I have now re-added the device-specific channel selection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still many changes here for a PR only intended to update e-h (as per the title).
Could you only do that and do the rest in separate PRs, please?
156564d
to
389d5bc
Compare
e52dd96
to
d809bfc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your work. Other than the comments this looks good to me now.
d809bfc
to
e6cc9d7
Compare
e6cc9d7
to
904325d
Compare
No description provided.